fix(admin): attach usage-tracking headers to Firestore requests - #308
Conversation
There was a problem hiding this comment.
Code Review
This pull request ensures that Firestore requests carry the SDK's usage-tracking headers (X-Firebase-Client and X-Goog-Api-Client) by extracting them into a shared utility and applying them to both FirebaseUserAgentClient and Firestore settings. The review feedback highlights two important improvements: first, instead of only applying tracking headers when no custom headers exist, they should be merged with any user-provided headers to prevent them from being completely omitted; second, the shared headers should be defined as a final variable rather than a getter to avoid redundant map allocations and regex evaluations on every HTTP request.
Coverage Report✅ Coverage 75.53% meets 40% threshold Total Coverage: 75.53% Package Breakdown
Minimum threshold: 40% |
| request.headers['X-Firebase-Client'] = 'fire-admin-dart/$packageVersion'; | ||
| request.headers['X-Goog-Api-Client'] = | ||
| 'gl-dart/$dartVersion fire-admin/$packageVersion'; | ||
| request.headers.addAll(firebaseUserAgentHeaders); |
There was a problem hiding this comment.
Is this semantically different than what was there before?
There was a problem hiding this comment.
No, same values — just centralized into one shared constant instead of duplicated at each call site.
… them across the two header maps
#307) * fix(ci): use allow-listed codecov-action@v4 ref instead of a SHA pin * test(ci): try pinning codecov-action to the actual v4 release SHA * Revert "test(ci): try pinning codecov-action to the actual v4 release SHA" This reverts commit 005b4a7. * feat(firestore): add Settings.headers to attach custom request headers * fix(firestore): append caller headers instead of overwriting existing X-Goog-Api-Client value * test(firestore): add unit tests for FirestoreRequestClient header merging * fix(firestore): address review feedback on Settings.headers * fix(admin): attach usage-tracking headers to Firestore requests (#308) * fix(admin): attach usage-tracking headers to Firestore requests * refactor(admin): share usage-tracking header values between FirebaseUserAgentClient and Firestore * fix(admin): avoid duplicate gl-dart tag when tagging Firestore requests * fix(admin): address review feedback, merge rather than replace user headers * docs(admin): wrap CHANGELOG entry to match surrounding style * refactor(admin): extract shared identity strings to avoid duplicating them across the two header maps * fix(ci): pin codecov-action to specific commit for stability * Revert "fix(ci): pin codecov-action to specific commit for stability" This reverts commit 9397148.
Firestorerequests never carried the SDK's usage-tracking headers (X-Firebase-Client,X-Goog-Api-Client) —FirestoreHttpClientbuilds its own HTTP client independently ofFirebaseApp.client/FirebaseUserAgentClient, so it was never wrapped.Uses the new
Settings.headersfrom #307 to attach the same headers, and extracts a sharedfirebaseUserAgentHeadershelper so the values aren't duplicated betweenFirebaseUserAgentClientandFirestore. Depends on #307 merging first.